[WPB-23806] (Un-)suspend apps if en-/disabled in the team.#5177
Open
[WPB-23806] (Un-)suspend apps if en-/disabled in the team.#5177
Conversation
zebot
added
the
ok-to-test
fisx
force-pushed
the
WPB-23806-_un-_suspend-apps-if-en-_disabled-in-the-team
branch
from
b270ee6 to
b1dd185
Compare
- add SetAccountStatus, GetAppIdsForTeam to BrigAPIAccess, - remove UserProfileFilter, - revert GetLocalUserProfilesFiltered to GetLocalUserProfiles, - introduce GetLocalAppProfilesOnly (dedicated & scalable), - instance SetFeatureConfig AppsConfig now (un-)suspends all apps in team.
fisx
force-pushed
the
WPB-23806-_un-_suspend-apps-if-en-_disabled-in-the-team
branch
from
b1dd185 to
67e1718
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Implements app account (un-)suspension when the team-level apps feature is toggled, by wiring Galley’s team feature updates to Brig account-status updates and adding supporting internal APIs.
Changes:
- Add Galley
AppsConfigfeature hook to suspend/unsuspend all app users in a team via Brig. - Add Brig internal endpoint to list app user IDs for a team, plus corresponding
BrigAPIAccessRPC calls. - Refactor Brig public “get app(s)” logic and extend
UserSubsystemwithGetLocalAppProfilesOnly, plus an integration test covering the suspend/unsuspend behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/galley/src/Galley/API/Teams/Features.hs | On apps feature enable/disable, fetch team app IDs from Brig and set their account status accordingly. |
| services/brig/src/Brig/API/Public.hs | Adjust app profile retrieval/listing to use the updated UserSubsystem API and a team-app-only path. |
| services/brig/src/Brig/API/Internal.hs | Add internal handler to return app user IDs for a team (for Galley to consume). |
| libs/wire-subsystems/test/unit/Wire/MockInterpreters/UserSubsystem.hs | Update mock interpreter for new UserSubsystem constructors. |
| libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs | Implement GetLocalUserProfiles and new GetLocalAppProfilesOnly using AppStore. |
| libs/wire-subsystems/src/Wire/UserSubsystem.hs | Remove UserProfileFilter API and add GetLocalAppProfilesOnly to the effect API. |
| libs/wire-subsystems/src/Wire/BrigAPIAccess/Rpc.hs | Add RPC calls for listing team app IDs and setting account status. |
| libs/wire-subsystems/src/Wire/BrigAPIAccess.hs | Expose new Brig API access operations in the effect. |
| libs/wire-api/src/Wire/API/Routes/Internal/Brig.hs | Add the internal route definition for GET /i/teams/:tid/apps. |
| integration/test/Test/FeatureFlags/Apps.hs | Add integration test asserting apps are suspended on disable and reactivated on enable. |
| changelog.d/3-bug-fixes/WPB-23806-_un-_suspend-apps-if-en-_disabled-in-the-team | Changelog entry for the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit 82eb5b3.
battermann
requested changes
Apr 16, 2026
Contributor
battermann
left a comment
battermann
left a comment
There was a problem hiding this comment.
If apps are suspended will they be removed from their conversations? (I hope not, so just checking)
Otherwise I would prefer to replace the zipWith with something more robust.
| GetLocalUserProfilesFiltered :: UserProfileFilter -> Local [UserId] -> UserSubsystem m [UserProfile] | ||
| GetLocalUserProfiles :: Local [UserId] -> UserSubsystem m [UserProfile] | ||
| -- | Get profiles for all app users in a team, touching only the apps table (efficient). | ||
| GetLocalAppProfilesOnly :: Local TeamId -> UserSubsystem m [UserProfile] |
Contributor
There was a problem hiding this comment.
I would remove the "Only" suffix. But that is a super opinionated nit-pick.
Comment on lines
+511
to
+517
| appIds <- getAppIdsForTeam tid | ||
| -- NB: this will work as long as the only reason for suspending | ||
| -- apps is "payment plan expired", but should we ever introduce a | ||
| -- suspend button for team admins to let them temporarily disable | ||
| -- apps without deinstalling them, then we need to keep track of | ||
| -- the suspend reason and filter for the right one here. | ||
| for_ appIds $ \uid -> setAccountStatus uid newStatus |
Contributor
There was a problem hiding this comment.
Could we do this more efficiently on the DB level? (nit-pick)
Comment on lines
+508
to
+517
| let newStatus = case feat.status of | ||
| FeatureStatusEnabled -> Active | ||
| FeatureStatusDisabled -> Suspended | ||
| appIds <- getAppIdsForTeam tid | ||
| -- NB: this will work as long as the only reason for suspending | ||
| -- apps is "payment plan expired", but should we ever introduce a | ||
| -- suspend button for team admins to let them temporarily disable | ||
| -- apps without deinstalling them, then we need to keep track of | ||
| -- the suspend reason and filter for the right one here. | ||
| for_ appIds $ \uid -> setAccountStatus uid newStatus |
Contributor
There was a problem hiding this comment.
we could consider to only update the status if the feature status actually changes. Currently any update of the feature (e.g. lockstatus, config) would trigger re-applying the same user status to the apps. not sure if this is really an issue.
| getLocalAppProfilesOnlyImpl ltid = do | ||
| apps <- AppStore.getApps (tUnqualified ltid) | ||
| profiles <- getUserProfilesLocalPart Nothing (ltid $> map (.id) apps) | ||
| pure (zipWith injectPreloadedApp profiles apps) |
Contributor
There was a problem hiding this comment.
this (zipWith) seems brittle. what if apps and profiles are not ordered in the same way, or contain unequal number of elements?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://wearezeta.atlassian.net/browse/WPB-23806
Checklist
changelog.d